-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update tensorstore zarr writer to write sequences #12
Conversation
JesseMckinzie
commented
Jul 2, 2024
- Update tensorstore zarr writer to writer sequences
src/cpp/writer/tswriter.cpp
Outdated
switch(_dtype_code) | ||
{ | ||
case (1): { | ||
auto data_array = tensorstore::Array(py_image.mutable_unchecked<std::uint8_t, 1>().data(0), _image_shape, tensorstore::c_order); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think _image_shape
is correct here. What if the data we are writing is only a part of the image? IMO, it should be deduced from rows
, cols
...etc.
rows = Seq(0, br._Y - 1, 1) | ||
cols = Seq(0, br._X - 1, 1) | ||
layers = Seq(0, 0, 1) | ||
channels = Seq(0, 0, 1) | ||
tsteps = Seq(0, 0, 1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a test where you are writing two different part of the image. May be a 2D image where you write two part of the images separately.
Also, another 3D image, where you write the channels separately.
src/python/bfiocpp/tswriter.py
Outdated
def write_image_data( | ||
self, | ||
image_data: np.ndarray, | ||
rows: int, | ||
cols: int, | ||
layers: int, | ||
channels: int, | ||
tsteps: int, | ||
): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This interface should support 2D-5D images. We should not have a restriction of only 5D image
|
||
void TsWriterCPP::WriteImageData(py::array& py_image) { | ||
auto position = dimension_order.find("X"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since "YX" will always be in dimension_order
, this check can be improved.
Instead of this, check if dimenstion_order
is of length between 2 and 5 and ends with YX
. If not, throw an exception. Also, _x_index
is always 0
and '_y_indexis always
1`. These don't need to be member variable.
rows: int, | ||
cols: int, | ||
layers: Optional[int] = None, | ||
channels: Optional[int] = None, | ||
tsteps: Optional[int] = None, | ||
): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these type correct?